Skip to content

Conversation

@rjernst
Copy link
Member

@rjernst rjernst commented Feb 14, 2025

With Security Manager we have SecuredConfigFileSettingAccessPermission. This commit adds an entitlement equivalent. With each entry in files entitlement, a path_setting can now be used. The value may be an explicit setting, or a setting glob with a single *.

relates ES-10844

With Security Manager we have SecuredConfigFileSettingAccessPermission.
This commit adds an entitlement equivalent. With each entry in files
entitlement, a `path_setting` can now be used. The value may be an
explicit setting, or a setting glob with a single `*`.

relates ES-10844
@rjernst rjernst added :Core/Infra/Core Core issues without another label >refactoring auto-backport Automatically create backport pull requests when merged v8.18.1 v8.19.0 v9.0.1 v9.1.0 labels Feb 14, 2025
@rjernst rjernst requested a review from a team as a code owner February 14, 2025 23:17
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Feb 14, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

}
}

final class AbsolutePathFileData implements FileData {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I'll align these implementations completely once #122658 is merged.

Copy link
Contributor

@ldematte ldematte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One optional observation but LGTM

Path configDir,
Path[] dataDirs,
Path tempDir,
Function<String, String> settingResolver,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is why I think and interface is better than a record here; a record with 2 Function is just and interface in disguise. OK to keep *Dir() as simple getters though.
Not blocking.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still in favor of the record here as there is only a single implementation. The resolvers are just necessary due to Settings not being available in the entitlements lib

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Moritz, I prefer a record because it is "things" we are passing in. Creating an interface would require creating an implementation, but the record provides that.

@rjernst rjernst enabled auto-merge (squash) February 17, 2025 14:07
@rjernst rjernst merged commit cffbccb into elastic:main Feb 18, 2025
22 checks passed
rjernst added a commit to rjernst/elasticsearch that referenced this pull request Feb 18, 2025
With Security Manager we have SecuredConfigFileSettingAccessPermission.
This commit adds an entitlement equivalent. With each entry in files
entitlement, a `path_setting` can now be used. The value may be an
explicit setting, or a setting glob with a single `*`.

relates ES-10844
rjernst added a commit to rjernst/elasticsearch that referenced this pull request Feb 18, 2025
With Security Manager we have SecuredConfigFileSettingAccessPermission.
This commit adds an entitlement equivalent. With each entry in files
entitlement, a `path_setting` can now be used. The value may be an
explicit setting, or a setting glob with a single `*`.

relates ES-10844
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.18
8.x
9.0

rjernst added a commit to rjernst/elasticsearch that referenced this pull request Feb 18, 2025
With Security Manager we have SecuredConfigFileSettingAccessPermission.
This commit adds an entitlement equivalent. With each entry in files
entitlement, a `path_setting` can now be used. The value may be an
explicit setting, or a setting glob with a single `*`.

relates ES-10844
elasticsearchmachine pushed a commit that referenced this pull request Feb 18, 2025
With Security Manager we have SecuredConfigFileSettingAccessPermission.
This commit adds an entitlement equivalent. With each entry in files
entitlement, a `path_setting` can now be used. The value may be an
explicit setting, or a setting glob with a single `*`.

relates ES-10844
elasticsearchmachine pushed a commit that referenced this pull request Feb 18, 2025
With Security Manager we have SecuredConfigFileSettingAccessPermission.
This commit adds an entitlement equivalent. With each entry in files
entitlement, a `path_setting` can now be used. The value may be an
explicit setting, or a setting glob with a single `*`.

relates ES-10844
elasticsearchmachine pushed a commit that referenced this pull request Feb 18, 2025
With Security Manager we have SecuredConfigFileSettingAccessPermission.
This commit adds an entitlement equivalent. With each entry in files
entitlement, a `path_setting` can now be used. The value may be an
explicit setting, or a setting glob with a single `*`.

relates ES-10844
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged :Core/Infra/Core Core issues without another label >refactoring Team:Core/Infra Meta label for core/infra team v8.18.1 v8.19.0 v9.0.1 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants